Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New concepts allowed even if name alrady exists in imported ontologies #504

Conversation

francescalb
Copy link
Collaborator

@francescalb francescalb commented Nov 20, 2022

Description:

Previously, names with prefLababels already present in the ontology including imported ontologies were
not allowed to be added.

This is now changed so that it is allowed to use another preflabel, provided that the base_iri of the new
ontology is not equal to that of the imported ontology with the already existing prefLabel.

Test rewritten accordinngly.

Type of change:

  • Bug fix.
  • New feature.
  • Documentation update.

Checklist:

This checklist can be used as a help for the reviewer.

  • Is the code easy to read and understand?
  • Are comments for humans to read, not computers to disregard?
  • Does a new feature has an accompanying new test (in the CI or unit testing schemes)?
  • Has the documentation been updated as necessary?
  • Does this close the issue?
  • Is the change limited to the issue?
  • Are errors handled for all outcomes?
  • Does the new feature provide new restrictions on dependencies, and if so is this documented?

Comments:

@francescalb francescalb linked an issue Nov 20, 2022 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2022

Codecov Report

Merging #504 (50ebd64) into master (c414377) will not change coverage.
The diff coverage is 85.71%.

@@           Coverage Diff           @@
##           master     #504   +/-   ##
=======================================
  Coverage   63.06%   63.06%           
=======================================
  Files          16       16           
  Lines        3081     3081           
=======================================
  Hits         1943     1943           
  Misses       1138     1138           
Impacted Files Coverage Δ
ontopy/excelparser.py 79.82% <85.71%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jesper-friis
Copy link
Collaborator

jesper-friis commented Nov 21, 2022

There seems to be an issue with get_by_label_all(). If I run pytest tests/test_basic.py and add some print statements to the test, I get the below error message. The issue seems not to be related to this PR, but it would still be nice to fix it before merging to master.

It seems that it is caused by self.label_annotations being None. The simplest solution seems to update line 149 in ontology.py to

        self._label_annotations = []

This change shows another issue on ontology.py line 377. If annotations is an empty generator, then next(annotations) will raise a StopIteration exception. Maybe the easiest solution is to replace line 377 with

        try:
            entity = self.world.search(**{next(annotations): label})
        except StopIteration:
            entity = []
=================================== FAILURES ===================================
__________________________________ test_basic __________________________________

emmo = get_ontology("http://emmo.info/emmo-inferred#")

    def test_basic(emmo: "Ontology") -> None:
        from ontopy import get_ontology
        from ontopy.utils import LabelDefinitionError
    
        onto = get_ontology("onto.owl")
        onto.imported_ontologies.append(emmo)
        onto.base_iri = "http://emmo.info/examples/test#"
    
        # Add entity directly
        onto.new_entity("Hydrogen", emmo.Atom)
    
    
        print([onto.Hydrogen])
>       print(onto.get_by_label_all("Hydrogen"))

tests/test_basic.py:23: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = get_ontology("http://emmo.info/examples/test#"), label = 'Hydrogen'
label_annotations = None, prefix = None

    def get_by_label_all(self, label, label_annotations=None, prefix=None):
        """Like get_by_label(), but returns a list with all matching labels.
    
        Returns an empty list if no matches could be found.
        """
        if not isinstance(label, str):
            raise TypeError(
                f"Invalid label definition, " f"must be a string: {label!r}"
            )
        if " " in label:
            raise ValueError(
                f"Invalid label definition, {label!r} contains spaces."
            )
    
        if label_annotations is None:
>           annotations = (_.name for _ in self.label_annotations)
E           TypeError: 'NoneType' object is not iterable

../../../.envs/ontopy/lib/python3.10/site-packages/ontopy/ontology.py:371: TypeError
----------------------------- Captured stdout call -----------------------------
[onto.Hydrogen]
=========================== short test summary info ============================
FAILED tests/test_basic.py::test_basic - TypeError: 'NoneType' object is not ...
========================= 1 failed, 1 passed in 1.25s ==========================

@francescalb francescalb merged commit 5b38254 into master Nov 25, 2022
@francescalb francescalb deleted the 500-excel2onto-allow-to-use-preflabel-already-in-imported-ontologies branch November 25, 2022 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

excel2onto: allow to use prefLabel already in imported ontologies
3 participants